Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add optional deps #98

Merged
merged 21 commits into from
Feb 26, 2025
Merged

add optional deps #98

merged 21 commits into from
Feb 26, 2025

Conversation

AnthonyLim23
Copy link
Collaborator

@AnthonyLim23 AnthonyLim23 commented Feb 24, 2025

This PR changes gofit to be an optional dependency. I have edited the tests such that:

  • Any code that doesn't use gofit is run with and without
  • Code that uses gofit is only run when its installed
  • Test that the gofit error is given if its not installed

I have written the code so the gofit fitting engine gives an error on initialisation if gofit is not installed.

@AnthonyLim23 AnthonyLim23 added the enhancement New feature or request label Feb 25, 2025
Comment on lines 36 to 46
Creates the scipy curve fit engine class
Stores useful information about each fit
:param name: name of the fit engine
:param x_data: original x data (can fit to an interpolation)
:param y_data: original y data (can fit to an interpolation)
:param e_data: original e data (can fit to an interpolation)
:param lower: the lower bounds for the fit parameters
:param upper: the upper bounds for the fit parameters
:param samples: the number of samples to use in multistart
:param max_iterations: the maximum number of iterations for the fit
"""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be worth updating this docstring to reflect the fact that the gofit engine isn't created in this case?

@jess-farmer
Copy link
Collaborator

All tests run as expected for me, just a couple of comments on docstrings

@jess-farmer jess-farmer merged commit a7862b9 into main Feb 26, 2025
15 checks passed
@jess-farmer jess-farmer deleted the 89_gofit_dep branch February 26, 2025 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants